Skip to content

Conversation

lolbinarycat
Copy link
Contributor

@lolbinarycat lolbinarycat commented Aug 26, 2025

fixes #138251

cc @notriddle

@rustbot rustbot added A-rustdoc-search Area: Rustdoc's search feature S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. labels Aug 26, 2025
@rust-log-analyzer

This comment has been minimized.

@lolbinarycat lolbinarycat force-pushed the rustdoc-search-trait-parent branch from 738dcc9 to 8444219 Compare August 26, 2025 19:35
@lolbinarycat
Copy link
Contributor Author

sometimes traitParent is not set properly for cross-crate implementations (eg. alloc::str::Bytes::last is not getting linked to Iterator::last).

this has been fixed (and removed from the PR description), i just needed to also be checking Cache::external_paths when resolving trait_parent_idx.

@rust-log-analyzer

This comment has been minimized.

@lolbinarycat
Copy link
Contributor Author

Right, I just remembered why my original design had its own deduplication pass, instead of leveraging the existing deduplication pass:

if the user searches for char::from, we should show them both ascii::Char::from and char::from. its only when a search matches the actual trait item (in this case, From::from) where we should perform this deduplication.

@lolbinarycat lolbinarycat force-pushed the rustdoc-search-trait-parent branch from 8444219 to 0574e11 Compare August 30, 2025 20:21
@rust-log-analyzer

This comment has been minimized.

@lolbinarycat lolbinarycat force-pushed the rustdoc-search-trait-parent branch from 0574e11 to 0c19f15 Compare August 30, 2025 20:45
@lolbinarycat
Copy link
Contributor Author

I believe I managed to get it working using notriddle's idea for how the search index data should be structured, but with my original idea of having two deduplication passes.

@rust-log-analyzer

This comment has been minimized.

@lolbinarycat lolbinarycat force-pushed the rustdoc-search-trait-parent branch from 0c19f15 to a78352a Compare August 30, 2025 20:56
@rust-log-analyzer

This comment has been minimized.

@lolbinarycat
Copy link
Contributor Author

ok so apparently some impls for unnamable traits get lowered and then filtered out later? which seems mildly wasteful but ig in the meantime i need to remove this debug assertion.

@lolbinarycat lolbinarycat force-pushed the rustdoc-search-trait-parent branch from a78352a to 2cf6317 Compare August 31, 2025 03:19
@rust-log-analyzer

This comment has been minimized.

@lolbinarycat

This comment was marked as resolved.

@rust-log-analyzer

This comment has been minimized.

@lolbinarycat lolbinarycat force-pushed the rustdoc-search-trait-parent branch from 25836ae to 8f00bd5 Compare September 5, 2025 17:47
@lolbinarycat lolbinarycat marked this pull request as ready for review September 5, 2025 19:04
@rustbot
Copy link
Collaborator

rustbot commented Sep 5, 2025

r? @notriddle

rustbot has assigned @notriddle.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 5, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 5, 2025

Some changes occurred in GUI tests.

cc @GuillaumeGomez

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha, @lolbinarycat

@lolbinarycat lolbinarycat changed the title WIP: don't show multiple instances of the same trait item in search If a trait item appears in rustdoc search, hide the corrosponding impl items Sep 5, 2025
@bors
Copy link
Collaborator

bors commented Sep 19, 2025

☔ The latest upstream changes (presumably #146700) made this pull request unmergeable. Please resolve the merge conflicts.

@rust-log-analyzer

This comment has been minimized.

@lolbinarycat lolbinarycat force-pushed the rustdoc-search-trait-parent branch from b9e60a2 to 84a6282 Compare September 22, 2025 17:12
@lolbinarycat lolbinarycat force-pushed the rustdoc-search-trait-parent branch from 84a6282 to 1adb51f Compare September 27, 2025 17:06
@rust-log-analyzer

This comment has been minimized.

@lolbinarycat
Copy link
Contributor Author

well that's a new error...

@rustbot rustbot added A-CI Area: Our Github Actions CI A-testsuite Area: The testsuite used to check the correctness of rustc T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Oct 2, 2025
@notriddle
Copy link
Contributor

I've pushed a commit onto your branch that I'm pretty sure will fix that.

After running these two commands...

2025-10-01T19:14:09.9965297Z + python3 ../x.py doc bootstrap
2025-10-01T19:14:32.7233881Z + RUSTDOCFLAGS=--document-private-items --document-hidden-items python3 ../x.py doc compiler --stage 1

... the CI run crashed in stage0, which contains beta's bootstrap toolchain, not the build of cargo and rustdoc that's actually in the PR.

Command `/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo doc --target x86_64-unknown-linux-gnu -Zbinary-dep-depinfo -j 4 -Zroot-dir=/checkout --locked --color always --release --features llvm --manifest-path /checkout/compiler/rustc/Cargo.toml -Zskip-rustdoc-fingerprint --no-deps -Zrustdoc-map -p rustc-main -p rustc_abi -p rustc_arena -p rustc_ast -p rustc_ast_ir -p rustc_ast_lowering -p rustc_ast_passes -p rustc_ast_pretty -p rustc_attr_parsing -p rustc_baked_icu_data -p rustc_borrowck -p rustc_builtin_macros -p rustc_codegen_llvm -p rustc_codegen_ssa -p rustc_const_eval -p rustc_data_structures -p rustc_driver -p rustc_driver_impl -p rustc_error_codes -p rustc_error_messages -p rustc_errors -p rustc_expand -p rustc_feature -p rustc_fluent_macro -p rustc_fs_util -p rustc_graphviz -p rustc_hashes -p rustc_hir -p rustc_hir_analysis -p rustc_hir_id -p rustc_hir_pretty -p rustc_hir_typeck -p rustc_incremental -p rustc_index -p rustc_index_macros -p rustc_infer -p rustc_interface -p rustc_lexer -p rustc_lint -p rustc_lint_defs -p rustc_llvm -p rustc_log -p rustc_macros -p rustc_metadata -p rustc_middle -p rustc_mir_build -p rustc_mir_dataflow -p rustc_mir_transform -p rustc_monomorphize -p rustc_next_trait_solver -p rustc_parse -p rustc_parse_format -p rustc_passes -p rustc_pattern_analysis -p rustc_privacy -p rustc_proc_macro -p rustc_public -p rustc_public_bridge -p rustc_query_impl -p rustc_query_system -p rustc_resolve -p rustc_sanitizers -p rustc_serialize -p rustc_session -p rustc_span -p rustc_symbol_mangling -p rustc_target -p rustc_thread_pool -p rustc_trait_selection -p rustc_traits -p rustc_transmute -p rustc_ty_utils -p rustc_type_ir -p rustc_type_ir_macros -p rustc_windows_rc [workdir=/checkout]` failed with exit code 101

This implies that the problem is just another variant of #147142 where a single docs directory is built from two different versions of rustdoc.

Copy link
Contributor

@notriddle notriddle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Squash, then merge?

View changes since this review

@lolbinarycat lolbinarycat force-pushed the rustdoc-search-trait-parent branch from 4e83303 to c7d6857 Compare October 2, 2025 20:29
@lolbinarycat
Copy link
Contributor Author

alright, I think this is probably a logical set of commits (maybe the first two could be squashed together).

i wanted to keep removeIdxListAsc in a separate commit in case we want to revert it in the future

@notriddle
Copy link
Contributor

@bors r=GuillaumeGomez,notriddle

@bors
Copy link
Collaborator

bors commented Oct 3, 2025

📌 Commit c7d6857 has been approved by GuillaumeGomez,notriddle

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 3, 2025
@bors
Copy link
Collaborator

bors commented Oct 3, 2025

⌛ Testing commit c7d6857 with merge dd09100...

@bors
Copy link
Collaborator

bors commented Oct 3, 2025

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez,notriddle
Pushing dd09100 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 3, 2025
@bors bors merged commit dd09100 into rust-lang:master Oct 3, 2025
11 checks passed
@rustbot rustbot added this to the 1.92.0 milestone Oct 3, 2025
Copy link
Contributor

github-actions bot commented Oct 3, 2025

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 3b8665c (parent) -> dd09100 (this PR)

Test differences

Show 2 test diffs

2 doctest diffs were found. These are ignored, as they are noisy.

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard dd091003ace19d9556c647d87f7a9cd1e8dcc17e --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. dist-android: 1624.9s -> 1110.3s (-31.7%)
  2. dist-apple-various: 4195.7s -> 3334.0s (-20.5%)
  3. pr-check-2: 2455.5s -> 2151.1s (-12.4%)
  4. x86_64-gnu-llvm-20-1: 3631.2s -> 3194.7s (-12.0%)
  5. i686-gnu-nopt-1: 8047.5s -> 7135.5s (-11.3%)
  6. dist-aarch64-apple: 7123.4s -> 6316.4s (-11.3%)
  7. aarch64-gnu-llvm-20-2: 2443.2s -> 2167.1s (-11.3%)
  8. dist-x86_64-windows-gnullvm: 4838.2s -> 5382.5s (11.3%)
  9. x86_64-gnu: 7750.3s -> 6972.8s (-10.0%)
  10. dist-x86_64-apple: 7448.6s -> 6740.6s (-9.5%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (dd09100): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (secondary -1.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-1.5% [-1.5%, -1.5%] 1
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 472.69s -> 470.825s (-0.39%)
Artifact size: 387.73 MiB -> 387.79 MiB (0.02%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CI Area: Our Github Actions CI A-rustdoc-search Area: Rustdoc's search feature A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Blanket impls cause bloated search results in rustdoc
7 participants